[#209] View counts for storyline and plot pages#217
Conversation
- Migration: page_views table with session dedup, view_count column on storylines, atomic increment_view_count RPC function - API: POST /api/views (insert + dedup per session/hour), GET /api/views (returns denormalized count) - ViewCount component: SVG eye icon + compact number formatting (1.2k, 1M) - ViewTracker component: fire-and-forget POST on page mount with sessionStorage-based session ID - Display: home page story cards, story detail header, writer dashboard - No performance impact on page load (tracking is async/non-blocking) Fixes #209 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The storyline-level view count pieces are in place, but the PR does not actually implement plot-level tracking yet. That leaves the main requirement from issue #209 incomplete.
Findings
- [high] Plot-level views are never recorded
- File:
src/app/story/[storylineId]/page.tsx:127 - File:
src/components/ViewCount.tsx:89 - Suggestion: mount
ViewTrackerwith a non-nullplotIndexat the actual plot-level page/view surface, or otherwise wire the client so individual plot views generatePOST /api/viewsrequests withplotIndexpopulated. Right now the only mounted tracker is<ViewTracker storylineId={id} />, so every inserted row hasplot_index = NULLand the plot-level path is dead code.
- File:
Decision
Requesting changes because issue #209 explicitly requires tracking page views for both storylines and individual plots in Supabase, and this implementation only records storyline views.
Each PlotEntry now mounts a ViewTracker with plotIndex, so individual plot views are recorded in page_views (not just storyline-level). Addresses T2a review feedback on PR #217. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The follow-up change wires ViewTracker into each rendered plot entry, so the plot-level tracking path is now actually exercised. The PR now matches issue #209 for storyline and individual-plot view recording, and the updated head passes CI.
Findings
- None.
Decision
Approving because the previously missing per-plot tracking path is now connected and the required display surfaces and API pieces are in place.
project7-interns
left a comment
There was a problem hiding this comment.
T2b: APPROVE
Clean implementation of view tracking with solid architecture:
- Migration: Proper table design with dedup index, RLS, and atomic increment RPC
- API: Good session-based dedup (1hr window), correct NULL handling for plot_index, service-role writes
- ViewCount: React Query with server-provided initialData avoids layout flash, 2-min staleTime is reasonable
- ViewTracker: SSR-safe sessionStorage + crypto.randomUUID(), fire-and-forget with query invalidation on new views
- Plot-level tracking: Each PlotEntry mounts its own ViewTracker with plotIndex — correctly addressed after T2a's feedback
Minor note (non-blocking): formatViewCountDashboard in the dashboard doesn't handle the 10M+ tier that formatViewCount in ViewCount.tsx does — cosmetic inconsistency only.
Summary
page_viewstable with session dedup index,view_countdenormalized column onstorylines,increment_view_countPostgres RPC functionPOST /api/viewsrecords views with 1-per-session-per-hour dedup;GET /api/views?storylineId=Nreturns countsessionStorage-based session ID, non-blockingAcceptance Criteria
Test plan
deduplicated: true)Fixes #209
🤖 Generated with Claude Code